[HUDI-2028] Implement RockDbBasedMap as an alternate to DiskBasedMap in ExternalSpillableMap #3117

rmahindra123 · 2021-06-20T04:52:04Z

What is the purpose of the pull request

This pull request adds a new alternative based on RockDb for the Disk Based Map that is used within the ExternalSpillableMap. Our benchmark results shows that RockDb may improve performance significantly when the data set is large while available memory may be scarce. RockDb supports compression, efficient memory usage and native library, that may be more efficient in certain situations. By default, disk based map will be used, and a config change will be required to enable rocksDb.

In this PR, the rocksDB support is only enabled for HoodieMergeHandle, and a subsequent PR will extend it to all consumers of ExternalSpillableMap (tracked here HUDI-2044)

Brief change log

Adds a new alternative based on RockDb for the Disk Based Map that is used within the ExternalSpillableMap.
The support is currently added only for HoodieMergeHandle

Verify this pull request

This change added tests and can be verified as follows:

Added the unit test in TestSpillableRocksDBBasedMap
Updated the test for TestExternalSpillableMap

hudi-bot · 2021-06-20T04:55:59Z

CI report:

72a3594 Azure: SUCCESS
231b679 Azure: PENDING

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run travis re-run the last Travis build
@hudi-bot run azure re-run the last Azure build

codecov-commenter · 2021-06-20T05:00:09Z

Codecov Report

Merging #3117 (72a3594) into master (c08fbb4) will decrease coverage by 1.72%.
The diff coverage is 80.19%.

@@             Coverage Diff              @@
##             master    #3117      +/-   ##
============================================
- Coverage     46.01%   44.29%   -1.73%     
+ Complexity     5306     4615     -691     
============================================
  Files           911      826      -85     
  Lines         39476    36669    -2807     
  Branches       4254     3949     -305     
============================================
- Hits          18166    16243    -1923     
+ Misses        19456    18674     -782     
+ Partials       1854     1752     -102

Flag	Coverage Δ
hudicli	`39.95% <ø> (ø)`
hudiclient	`16.44% <14.28%> (-14.00%)`	⬇️
hudicommon	`47.74% <85.10%> (+0.18%)`	⬆️
hudiflink	`59.94% <ø> (-1.40%)`	⬇️
hudihadoopmr	`51.29% <ø> (ø)`
hudisparkdatasource	`67.06% <ø> (+0.53%)`	⬆️
hudisync	`54.05% <ø> (+2.31%)`	⬆️
huditimelineservice	`64.36% <ø> (ø)`
hudiutilities	`58.40% <ø> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ain/java/org/apache/hudi/io/HoodieMergeHandle.java	`0.00% <0.00%> (ø)`
...org/apache/hudi/common/util/SpillableMapUtils.java	`86.84% <ø> (ø)`
...java/org/apache/hudi/config/HoodieWriteConfig.java	`17.03% <20.00%> (+0.03%)`	⬆️
...he/hudi/common/util/collection/BitCaskDiskMap.java	`80.88% <75.00%> (ø)`
...apache/hudi/common/util/collection/RocksDBDAO.java	`75.12% <81.81%> (+0.26%)`	⬆️
...he/hudi/common/util/collection/RocksDbDiskMap.java	`85.10% <85.10%> (ø)`
...i/common/util/collection/ExternalSpillableMap.java	`77.45% <85.71%> (+6.02%)`	⬆️
...common/table/log/HoodieMergedLogRecordScanner.java	`82.35% <100.00%> (ø)`
.../hudi/common/util/collection/LazyFileIterable.java	`74.41% <100.00%> (ø)`
...e/hudi/common/util/collection/RocksDBBasedMap.java	`0.00% <0.00%> (-39.29%)`	⬇️
... and 151 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c08fbb4...72a3594. Read the comment docs.

…eDeltaStreamer (apache#3110)

…tant time before flushing (apache#3123)

…flink compactor (apache#3124)

hudi-common/src/main/java/org/apache/hudi/common/util/collection/RocksDBDAO.java

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

…ethod have wrong filter condition (apache#3109)

…culative execution is enabled (apache#3093) * unit tests added

…thsOfInstant (apache#3125) Co-authored-by: 喻兆靖 <[email protected]>

…Table is enabled. (apache#3079)

…te instants from the dataset timeline. (apache#3082)

…eClient$commitstats (apache#3050)

…e#3135)

…tionPlanOperator (apache#3105) Co-authored-by: 喻兆靖 <[email protected]>

…apache#3141)

…he#3137)

…rtition Value (apache#3133)

…apache#3111) [HUDI-2069] KafkaAvroSchemaDeserializer should get sourceSchema passed instead using Reflection

…tadata Safely (apache#3138) Co-authored-by: 喻兆靖 <[email protected]>

…can not assign initially (apache#3148)

…HMS in different regions (apache#2542) [global-hive-sync-tool] Add a global hive sync tool to sync hudi table across clusters. Add a way to rollback the replicated time stamp if we fail to sync or if we partly sync Co-authored-by: Jagmeet Bali <[email protected]>

nsivabalan

I see we have more test coverage in TestExternalSpillableMap. Can we parametrize the tests that are applicable and run those for both type of spillable maps.

In general, do we have tests around diff values for maxInMemorySizeInBytes for external spillable map. If not, do you think we can add them while we are at this.

hudi-common/src/main/java/org/apache/hudi/common/util/collection/SpillableDiskMap.java

hudi-common/src/main/java/org/apache/hudi/common/util/collection/ExternalSpillableMap.java

hudi-common/src/main/java/org/apache/hudi/common/util/collection/SpillableDiskMap.java

hudi-common/src/main/java/org/apache/hudi/common/util/collection/SpillableRocksDBBasedMap.java

nsivabalan · 2021-06-28T02:07:30Z

hudi-common/src/main/java/org/apache/hudi/common/util/collection/SpillableRocksDBBasedMap.java

+
+  @Override
+  public Stream<R> valueStream() {
+    return keySet.stream().sorted().sequential().map(valueMetaData -> (R) get(valueMetaData));


am not very conversant w/ this spillable map in general.
May I know is there a necessity to sort here? I don't see external spillable map in master does any sort for valueStream. also, for putAll() I don't see any validation as such for sorted entries.

Good catch. I made it consistent with DiskBasedMap, but in disk based map we do it to sort the offsets for ensuring sequential reads. But for rocksDb, we do not need to do it and leave it to RocksDb to do it.

hudi-common/src/main/java/org/apache/hudi/common/util/collection/SpillableRocksDBBasedMap.java

nsivabalan · 2021-06-28T02:18:21Z

...ommon/src/test/java/org/apache/hudi/common/util/collection/TestSpillableRocksDBBasedMap.java

+    while (itr.hasNext()) {
+      HoodieRecord<? extends HoodieRecordPayload> rec = itr.next();
+      oRecords.add(rec);
+      assert recordKeys.contains(rec.getRecordKey());


minor. why are we not testing for value equality. is there any particular reason ?

...ommon/src/test/java/org/apache/hudi/common/util/collection/TestSpillableRocksDBBasedMap.java

…putFormat#MergeIterator to avoid StackOverflow (apache#3159)

…he#3167)

)

…he#3092)

…he#3168) Co-authored-by: 喻兆靖 <[email protected]>

…3166)

…#3178)

…et io for flink batch compaction (apache#3169)

rmahindra123 · 2021-06-29T19:20:04Z

I see we have more test coverage in TestExternalSpillableMap. Can we parametrize the tests that are applicable and run those for both type of spillable maps.

I had tests for TestExternalSpillableMap in the stacked diff, anyway added it here.

In general, do we have tests around diff values for maxInMemorySizeInBytes for external spillable map. If not, do you think we can add them while we are at this.

We do test if the right amount of keys were in memory and the right amount was spilled over. Wondering how different values of maxInMemorySizeInBytes will help?

…code null value (apache#3176)

Co-authored-by: 喻兆靖 <[email protected]>

* [HUDI-1944] Support Hudi to read from committed offset * [HUDI-1944] Adding group option to KafkaResetOffsetStrategies * [HUDI-1944] Update Exception msg

Co-authored-by: 喻兆靖 <[email protected]>

…to rm_rocks_db

rmahindra123 · 2021-06-30T21:10:14Z

Moved to #3194

rmahindra123 · 2021-06-30T21:10:55Z

Moved to #3194

Implement RockDbBasedMap as an alternate to DiskBasedMap in SpillableMap

29a53c6

rmahindra321 mentioned this pull request Jun 20, 2021

[HUDI-2028] Implement RockDbBasedMap as an alternate to DiskBasedMap in SpillableMap #3112

Closed

xushiyan and others added 5 commits June 20, 2021 14:36

[MINOR] Put Azure cache tasks first (apache#3118)

e41f13f

[HUDI-1248] Increase timeout for deltaStreamerTestRunner in TestHoodi…

429e9fb

…eDeltaStreamer (apache#3110)

[HUDI-2049] StreamWriteFunction should wait for the next inflight ins…

adf1679

…tant time before flushing (apache#3123)

[HUDI-2050] Support rollback inflight compaction instances for batch …

f8d9242

…flink compactor (apache#3124)

[HUDI-1776] Support AlterCommand For Hoodie (apache#3086)

4fd8a88

rmahindra123 commented Jun 21, 2021

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/common/util/collection/RocksDBDAO.java Show resolved Hide resolved

Fix iterator for rocks db

93dad05

rmahindra123 commented Jun 21, 2021

View reviewed changes

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java Outdated Show resolved Hide resolved

swuferhong and others added 18 commits June 21, 2021 17:53

[HUDI-2043] HoodieDefaultTimeline$filterPendingCompactionTImeline() m…

cb5cd35

…ethod have wrong filter condition (apache#3109)

[HUDI-2031] JVM occasionally crashes during compaction when spark spe…

7bd517a

…culative execution is enabled (apache#3093) * unit tests added

[HUDI-2047] Ignore FileNotFoundException in WriteProfiles #getWritePa…

5db37c2

…thsOfInstant (apache#3125) Co-authored-by: 喻兆靖 <[email protected]>

[HUDI-1883] Support Truncate Table For Hoodie (apache#3098)

69c0d9e

[HUDI-2013] Removed option to fallback to file listing when Metadata …

062d5ba

…Table is enabled. (apache#3079)

[HUDI-1717] Metadata Reader should merge all the un-synced but comple…

11e64b2

…te instants from the dataset timeline. (apache#3082)

[HUDI-1988] FinalizeWrite() been executed twice in AbstractHoodieWrit…

3fb59dd

…eClient$commitstats (apache#3050)

[HUDI-2054] Remove the duplicate name for flink write pipeline (apach…

2687eab

…e#3135)

[HUDI-1826] Add ORC support in HoodieSnapshotExporter (apache#3130)

43b9c1f

[HUDI-2038] Support rollback inflight compaction instances for Compac…

380518e

…tionPlanOperator (apache#3105) Co-authored-by: 喻兆靖 <[email protected]>

Fix checkstyle issues

dd248a3

[HUDI-2064] Fix TestHoodieBackedMetadata#testOnlyValidPartitionsAdded (…

e039e0f

…apache#3141)

[HUDI-2061] Incorrect Schema Inference For Schema Evolved Table (apac…

7e50f9a

…he#3137)

[HUDI-2053] Insert Static Partition With DateType Return Incorrect Pa…

84dd3ca

…rtition Value (apache#3133)

[HUDI-2069] Fix KafkaAvroSchemaDeserializer to not rely on reflection (…

b328555

…apache#3111) [HUDI-2069] KafkaAvroSchemaDeserializer should get sourceSchema passed instead using Reflection

[HUDI-2062] Catch FileNotFoundException in WriteProfiles #getCommitMe…

218f2a6

…tadata Safely (apache#3138) Co-authored-by: 喻兆靖 <[email protected]>

[HUDI-2068] Skip the assign state for SmallFileAssign when the state …

e64fe55

…can not assign initially (apache#3148)

nsivabalan reviewed Jun 28, 2021

View reviewed changes

danny0405 and others added 9 commits June 28, 2021 16:03

[HUDI-2074] Use while loop instead of recursive call in MergeOnReadIn…

d24341d

…putFormat#MergeIterator to avoid StackOverflow (apache#3159)

[MINOR] Drop duplicate keygenerator class configuration setting (apac…

9e61dad

…he#3167)

[HUDI-2067] Sync FlinkOptions config to FlinkStreamerConfig (apache#3151

34fc8a8

)

[HUDI-1910] Commit Offset to Kafka after successful Hudi commit (apac…

039aeb6

…he#3092)

[HUDI-2084] Resend the uncommitted write metadata when start up (apac…

37b7c65

…he#3168) Co-authored-by: 喻兆靖 <[email protected]>

[HUDI-2081] Move schema util tests out from TestHiveSyncTool (apache#…

0749cc8

…3166)

[HUDI-2094] Supports hive style partitioning for flink writer (apache…

b8a8f57

…#3178)

[HUDI-2097] Fix Flink unable to read commit metadata error (apache#3180)

5a7d1b3

[HUDI-2085] Support specify compaction paralleism and compaction targ…

f665db0

…et io for flink batch compaction (apache#3169)

rmahindra123 and others added 13 commits June 29, 2021 13:49

Address reviewer comments

6d4b556

Address reviewer comments

72a3594

[HUDI-2092] Fix NPE caused by FlinkStreamerConfig#writePartitionUrlEn…

202887b

…code null value (apache#3176)

[HUDI-2006] Adding more yaml templates to test suite (apache#3073)

5564c7e

[HUDI-2103] Add rebalance before index bootstrap (apache#3185)

1cbf43b

Co-authored-by: 喻兆靖 <[email protected]>

[HUDI-1944] Support Hudi to read from committed offset (apache#3175)

94f0f40

* [HUDI-1944] Support Hudi to read from committed offset * [HUDI-1944] Adding group option to KafkaResetOffsetStrategies * [HUDI-1944] Update Exception msg

[HUDI-2052] Support load logFile in BootstrapFunction (apache#3134)

07e93de

Co-authored-by: 喻兆靖 <[email protected]>

Implement RockDbBasedMap as an alternate to DiskBasedMap in SpillableMap

91427f3

Fix iterator for rocks db

26ed037

Fix checkstyle issues

6ca64ba

Address reviewer comments

4eb25d0

Address reviewer comments

e45577b

Merge branch 'rm_rocks_db' of https://github.com/rmahindra123/hudi in…

231b679

…to rm_rocks_db

rmahindra123 mentioned this pull request Jun 30, 2021

[HUDI-2028] Implement RockDbBasedMap as an alternate to DiskBasedMap in ExternalSpillableMap #3194

Merged

rmahindra123 closed this Jun 30, 2021

rmahindra123 deleted the rm_rocks_db branch June 30, 2021 21:10

rmahindra123 mentioned this pull request Jul 21, 2021

[HUDI-2044] Integrate consumers with rocksDB and compression within External Spillable Map #3318

Merged

[HUDI-2028] Implement RockDbBasedMap as an alternate to DiskBasedMap in ExternalSpillableMap #3117

[HUDI-2028] Implement RockDbBasedMap as an alternate to DiskBasedMap in ExternalSpillableMap #3117

Uh oh!

Conversation

rmahindra123 commented Jun 20, 2021

What is the purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

hudi-bot commented Jun 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

codecov-commenter commented Jun 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

nsivabalan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nsivabalan Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

rmahindra123 Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nsivabalan Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rmahindra123 commented Jun 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rmahindra123 commented Jun 30, 2021

Uh oh!

rmahindra123 commented Jun 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

hudi-bot commented Jun 20, 2021 •

edited

Loading

codecov-commenter commented Jun 20, 2021 •

edited

Loading

nsivabalan left a comment •

edited

Loading

rmahindra123 commented Jun 29, 2021 •

edited

Loading